Bump python sdk version #827

eric-wang-1990 · 2024-10-14T23:09:25Z

Upgrade python sdk version from 0.17.0 to 0.36.0
Create DatabricksCredentialManager to managed credentials, under the hood use Config from databricks.sdk.core to manage the auth part which comes from the Python SDK.
For dbt we support 4 different auth mode:

token : When user explicitly passed in a token=xxxx
external_browser: When user pass in auth_type=oauth with clientid=xxx and no client secret
oauth-m2m: When user pass in clientid=xxx and clientsecret=yyy
azure-client-secret: When user pass in azure_client_id=xxx and azure_client_secret=yyy and your host is an azure host

For oauth-m2m and azure-client-secret we set a preferred auth sequence based on if the clientSecret starts with "dose", so we can reduce the number of false trying as much as possible.

Remove the set/get sharedPassword part, since the python sdk already handles that, but with one defect which I already raise a PR against: databricks/databricks-sdk-py#823

With this change one thing that will no longer work is if customer are using non-databricks OAuth client for U2M case it will not work, since the python sdk does not support modify redirectUrl/scopes.

Description

Checklist

I have run this code in development and it appears to resolve the stated issue
This PR includes tests, or tests are not required/relevant for this PR
I have updated the CHANGELOG.md and added information about my change to the "dbt-databricks next" section.

… config file (#724) Signed-off-by: Artur Peedimaa <[email protected]> Co-authored-by: Artur Peedimaa <[email protected]> Co-authored-by: Ben Cassell <[email protected]>

Signed-off-by: Dmitry Volodin <[email protected]> Co-authored-by: Ben Cassell <[email protected]>

Co-authored-by: Antoine <[email protected]>

Signed-off-by: Roy Dobbe [email protected] Co-authored-by: Roy Dobbe <[email protected]>

Signed-off-by: Kyle Valade <[email protected]> Co-authored-by: Kyle Valade <[email protected]> Co-authored-by: Ben Cassell <[email protected]>

…s into bump_python_sdk_version

jackyhu-db · 2024-11-15T01:54:21Z

dbt/adapters/databricks/credentials.py

-        self._lock.acquire()
-        try:
+        assert self._credentials_manager is not None, "Credentials manager is not set."
+        return self._credentials_manager


is lock still required?

I am just honoring the existing behavior. If there are multiple connections using the same DatabricksCredentials instance, there is a shared resource _config which can have concurrent write. @benc-db I think you introduce the original lock, is it still needed?

jackyhu-db · 2024-11-15T01:59:30Z

dbt/adapters/databricks/credentials.py

+                auth_type = (
+                    "external-browser" if not self.client_secret
+                    # if the client_secret starts with "dose" then it's likely using oauth-m2m
+                    else "oauth-m2m" if self.client_secret.startswith("dose")


can it wrapped in a method like is_databricks_m2m? And this checks still a little bit tricky, do we really want to infer it from secret format? Can it be explicitly specified by the config to decide whether azure or databricks m2m should be used?

+1 to what Jacky said. In design review we decided to not depend on secret format, but instead to a.) reference if a config flag is set to tell us to use AAD, and b.) if we fail with Databricks OAuth but succeed with AAD, emit a warning telling them to set the config flag.

jackyhu-db · 2024-11-15T02:00:52Z

Thanks @eric-wang-1990 for updating this. Can your rename the PR title to make it more readable?

benc-db · 2024-11-15T17:10:46Z

Hi Eric, since the last time we were looking at this PR, 1.9 has become the target in main. So please rebase and target main with PR.

benc-db

Please remove dependence on secret format, per internal design meeting.

… bump_python_sdk_version

eric-wang-1990 · 2024-11-16T01:05:16Z

dbt/adapters/databricks/credentials.py


-    def set_sharded_password(self, service_name: str, username: str, password: str) -> None:
-        max_size = MAX_NT_PASSWORD_SIZE
+    Without this, a local .netrc file in the user's home directory


Is it safe to remove it seems databricks cli will not use .netrc anymore? https://docs.databricks.com/en/dev-tools/cli/profiles.html @benc-db @jackyhu-db

I think .netrc is used by python requests which might be used by databricks-sdk code as well?

eric-wang-1990 · 2024-11-16T01:08:13Z

dbt/adapters/databricks/credentials.py

-        self._lock.acquire()
-        try:
+        assert self._credentials_manager is not None, "Credentials manager is not set."
+        return self._credentials_manager


I am just honoring the existing behavior. If there are multiple connections using the same DatabricksCredentials instance, there is a shared resource _config which can have concurrent write. @benc-db I think you introduce the original lock, is it still needed?

benc-db and others added 26 commits June 24, 2024 13:24

Add support for Serverless jobs / refactor api usage (#706)

6a17257

Merge branch 'main' into 1.9.latest

3cf4aba

Cleanup test warnings (#713)

4795063

Merge branch 'main' into 1.9.latest

bcab2a4

Fix dbt seed command error when seed file is partially defined in the…

855bb5e

… config file (#724) Signed-off-by: Artur Peedimaa <[email protected]> Co-authored-by: Artur Peedimaa <[email protected]> Co-authored-by: Ben Cassell <[email protected]>

Readd external type (#728)

17433f3

Upgrade to PySQL 3.2.0 (#729)

8e88384

Merge branch 'main' into 1.9.latest

3577f85

Merge branch 'main' into 1.9.latest

708cf60

bump python sdk version

25636de

update

12c077b

Extend Merge Capabilities (#739)

a35918b

Signed-off-by: Dmitry Volodin <[email protected]> Co-authored-by: Ben Cassell <[email protected]>

Merge branch 'main' into 1.9.latest

a2b8ce9

Forward porting latest 1.8 changes into 1.9 branch (#788)

b8486d1

Co-authored-by: Antoine <[email protected]>

Upgrade PySql to 3.4.0 (#790)

98177fe

Add custom constraint option (#792)

092296b

Signed-off-by: Roy Dobbe [email protected] Co-authored-by: Roy Dobbe <[email protected]>

Merge branch 'main' into 1.9.latest

dcbeb0c

Merge branch 'main' into 1.9.latest

60b487d

Behavior: Get column info from information_schema Part I (#808)

41c164e

Simple Iceberg support (#815)

3412461

Merge branch 'main' into 1.9.latest

103f1b1

fix merge issue

7e6b450

Merge branch 'main' into 1.9.latest

74c7862

Draft: #756 - implement python workflow submissions (#762)

0e821b0

Signed-off-by: Kyle Valade <[email protected]> Co-authored-by: Kyle Valade <[email protected]> Co-authored-by: Ben Cassell <[email protected]>

Behavior for external path (#823)

00dd9f8

Merge branch '1.9.latest' of github.com-ghec:databricks/dbt-databrick…

5b8d938

…s into bump_python_sdk_version

eric-wang-1990 requested review from andrefurlan-db, benc-db and rcypher-databricks as code owners October 14, 2024 23:09

eric-wang-1990 had a problem deploying to azure-prod October 14, 2024 23:21 — with GitHub Actions Failure

eric-wang-1990 had a problem deploying to azure-prod October 31, 2024 06:21 — with GitHub Actions Failure

eric-wang-1990 added 2 commits October 31, 2024 00:01

fix token test

737f021

test

8c8417c

eric-wang-1990 had a problem deploying to azure-prod November 14, 2024 18:07 — with GitHub Actions Failure

fix test, add lock

c4aa1a3

eric-wang-1990 had a problem deploying to azure-prod November 15, 2024 00:25 — with GitHub Actions Failure

eric-wang-1990 requested a review from jackyhu-db November 15, 2024 00:50

jackyhu-db reviewed Nov 15, 2024

View reviewed changes

benc-db requested changes Nov 15, 2024

View reviewed changes

update

8dcba15

eric-wang-1990 had a problem deploying to azure-prod November 16, 2024 00:24 — with GitHub Actions Error

eric-wang-1990 had a problem deploying to azure-prod November 16, 2024 00:25 — with GitHub Actions Error

eric-wang-1990 changed the base branch from 1.9.latest to main November 16, 2024 00:28

Merge branch 'main' of github.com-ghec:databricks/dbt-databricks into…

bfe05a9

… bump_python_sdk_version

eric-wang-1990 had a problem deploying to azure-prod November 16, 2024 00:43 — with GitHub Actions Failure

eric-wang-1990 commented Nov 16, 2024

View reviewed changes

update

55de168

eric-wang-1990 had a problem deploying to azure-prod November 16, 2024 01:08 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump python sdk version #827

Bump python sdk version #827

eric-wang-1990 commented Oct 14, 2024 •

edited

Loading

jackyhu-db Nov 15, 2024

eric-wang-1990 Nov 16, 2024

jackyhu-db Nov 15, 2024

benc-db Nov 15, 2024

jackyhu-db commented Nov 15, 2024

benc-db commented Nov 15, 2024

benc-db left a comment

eric-wang-1990 Nov 16, 2024

jackyhu-db Nov 16, 2024

eric-wang-1990 Nov 16, 2024

Bump python sdk version #827

Are you sure you want to change the base?

Bump python sdk version #827

Conversation

eric-wang-1990 commented Oct 14, 2024 • edited Loading

Description

Checklist

jackyhu-db Nov 15, 2024

Choose a reason for hiding this comment

eric-wang-1990 Nov 16, 2024

Choose a reason for hiding this comment

jackyhu-db Nov 15, 2024

Choose a reason for hiding this comment

benc-db Nov 15, 2024

Choose a reason for hiding this comment

jackyhu-db commented Nov 15, 2024

benc-db commented Nov 15, 2024

benc-db left a comment

Choose a reason for hiding this comment

eric-wang-1990 Nov 16, 2024

Choose a reason for hiding this comment

jackyhu-db Nov 16, 2024

Choose a reason for hiding this comment

eric-wang-1990 Nov 16, 2024

Choose a reason for hiding this comment

eric-wang-1990 commented Oct 14, 2024 •

edited

Loading